feat(stepfunctions-tasks): support for APIGW API: Invoke #11565

Sumeet-Badyal · 2020-11-19T00:06:24Z

feat(stepfunctions-tasks): support for APIGW API: Invoke

Implementation

Update package @aws-cdk/aws-stepfunctions-tasks to include support for ApiGateway Invoke
API as per documentation here:
https://docs.aws.amazon.com/step-functions/latest/dg/connect-api-gateway.html
https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-control-access-using-iam-policies-to-invoke-api.html

Includes support for the following Amazon Athena API calls:
Invoke

Closes #11565

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2020-11-19T00:06:27Z

shivlaks · 2020-11-19T02:43:57Z

@pahud feel free to give the open PR a review as well!

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

ayush987goyal

Thanks for this change! Some comments.

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

packages/@aws-cdk/aws-stepfunctions-tasks/test/apigateway/invoke.test.ts

packages/@aws-cdk/aws-stepfunctions-tasks/README.md

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

shivlaks · 2020-11-19T06:05:43Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+  /**
+   * Retreive data from a server at the specified resource
+   */


Suggested change

/**

* Retreive data from a server at the specified resource

*/

/** Retrieve data from a server at the specified resource */

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

shivlaks · 2020-11-19T06:39:44Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+    } else {
+      return {
+        Resource: integrationResourceArn('apigateway', 'invoke', this.integrationPattern),
+        Parameters: sfn.FieldUtils.renderObject({
+          ApiEndpoint: this.props.apiEndpoint,
+          Method: this.props.method,
+          Stage: this.props.stage,
+          Path: this.props.path,
+          AuthType: (this.props.authType ? this.props.authType.value : sfn.TaskInput.fromDataAt('$.AuthType').value) === '$' ? (this.props.authType ? this.props.authType.value : sfn.TaskInput.fromDataAt('$.AuthType').value) : 'NO_AUTH',
+        }),
+      };
+    }
+  }


there's a lot of duplicated code in this control flow chain from the if on down. I think we can simplify this to be a lot more succinct. let's refactor this please!

Pull request has been modified.

ayush987goyal · 2020-11-20T02:50:13Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+   * hostname of an API Gateway URL
+   * @example {ApiId}.execute-api.{region}.amazonaws.com
+   */
+  readonly apiEndpoint: string;


I think this should not be needed anymore.

shivlaks

would also love if @nija-at could take a look since he's the primary maintainer of the api-gateway module

shivlaks · 2020-11-23T18:06:53Z

packages/@aws-cdk/aws-stepfunctions-tasks/README.md

+
+const invokeJob = new tasks.ApiGatewayInvoke(stack, 'Invoke APIGW', {
+  api: restApi,
+  apiEndpoint: restApi.restApiId,


why does this need to be an input parameter as well? you're already accepting the restApi as input

shivlaks · 2020-11-23T18:07:51Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+   * hostname of an API Gateway URL
+   * @example {ApiId}.execute-api.{region}.amazonaws.com
+   */
+  readonly apiEndpoint: string;


the endpoint can be inferred from the api so you shouldn't need to make this an input anymore

shivlaks · 2020-11-23T18:09:48Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+   *   }
+   * },
+   */
+  readonly headers?: { [key: string]: any };


confirming to ensure we have the right type here: "Headers.$" : "$.input" is not valid correct?
i.e. the entire headers field cannot be allocated to a field in the state input

Anyone digging into this? Happy to try and help; looking forward to putting this functionality to use ;)

@eikeon - happy to have your help here!! :)

Is this pretty much the sole remaining thing holding up this pull request? Can save some time to attempt to dig into this headers issue later today

headers here probably should be similar to input here: packages/@aws-cdk/aws-stepfunctions-tasks/test/start-execution.test.ts and packages/@aws-cdk/aws-stepfunctions-tasks/lib/start-execution.ts -- and they both have the same type. Gotta run for a few hours, but will trace how headers is being used in this pull request compared to other working examples.

And https://docs.aws.amazon.com/step-functions/latest/dg/connect-api-gateway.html

readonly headers?: { [key: string]: string[] }

I also see a string value in one of the examples on that doc page: readonly headers?: { [key: string]: string[] | string }

shivlaks · 2020-11-23T18:10:54Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+
+  /**
+   * Name of the stage where the API is deployed to in API Gateway
+   * @default - Required for REST and $default for HTTP


nit: be sure to describe what $default means as the user might find that information useful in determining whether they need to set it or not.

shivlaks · 2020-11-23T18:11:14Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+
+  /**
+   * Authentication methods
+   * @default - NO_AUTH


Suggested change

* @default - NO_AUTH

* @default AuthType.NO_AUTH

shivlaks · 2020-11-25T02:07:52Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+  }
+
+  /**
+   * Gets the "execute-api" ARN


nit: I think the doc string should be a little more informative than the method name. Perhaps describe what that ARN is / what it's used for, show an example of what it looks like, etc.

shivlaks · 2020-11-25T02:09:55Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/apigateway/invoke.ts

+
+/**
+ * The authentication method used to call the endpoint
+ * @default NO_AUTH


enums don't have defaults, it's only where they're used that would

Suggested change

* @default NO_AUTH

shivlaks · 2020-11-25T02:11:09Z

packages/@aws-cdk/aws-stepfunctions-tasks/test/apigateway/integ.invoke.ts

+ * Stack verification steps:
+ * * aws stepfunctions start-execution --state-machine-arn <deployed state machine arn> : should return execution arn
+ * * aws stepfunctions describe-execution --execution-arn <exection-arn generated before> : should return status as SUCCEEDED and a query-execution-id
+ * * aws apigateway test-invoke-method --rest-api-id <value> --resource-id <value> --http-method <value>: should return the same response as the state machine


should return the same response as the state machine

nit: better to be explicit and clear about what that response is

shivlaks · 2020-11-25T02:14:52Z

packages/@aws-cdk/aws-stepfunctions-tasks/test/apigateway/integ.invoke.ts

+function helloCode(_event: any, _context: any, callback: any) {
+  return callback(undefined, {
+    statusCode: 200,
+    body: 'hello, world!',
+  });
+}


does this need to be a function? - why not just define it ahead of it's usage or inline it?

shivlaks · 2020-11-25T02:17:23Z

packages/@aws-cdk/aws-stepfunctions-tasks/test/apigateway/integ.invoke.ts

+ * Stack verification steps:
+ * * aws stepfunctions start-execution --state-machine-arn <deployed state machine arn> : should return execution arn
+ * * aws stepfunctions describe-execution --execution-arn <exection-arn generated before> : should return status as SUCCEEDED and a query-execution-id
+ * * aws apigateway test-invoke-method --rest-api-id <value> --resource-id <value> --http-method <value>: should return the same response as the state machine


should the rest-api-id also be an output? same question for all the inputs here. steps should include where they retrieved from. you might need to add outputs to the stack if they're not easily available.

Could you clarify what you mean by this? I am not sure how to get the rest-api-id.

Pull request has been modified.

aws-cdk-automation · 2020-12-03T19:54:42Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9cd40c7
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

ghost · 2021-01-17T12:48:25Z

Hi Guys, what's up with the feature ?

shivlaks · 2021-03-03T18:04:51Z

superseded by #13033

@mmcircle - efforts to land this have been resumed in a different PR (#13033). please follow that one for the current status of API Gateway service integration support.

@Sumeet-Badyal

…PIs (#13033) feat(stepfunctions-tasks): Support calling APIGW REST and HTTP APIs Taking ownership of the original PR #11565 by @Sumeet-Badyal API as per documentation here: https://docs.aws.amazon.com/step-functions/latest/dg/connect-api-gateway.html https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-control-access-using-iam-policies-to-invoke-api.html closes #11566 closes #11565 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

feat: add APIGW Invoke to SFN-tasks

51e47d8

shivlaks self-assigned this Nov 19, 2020

shivlaks added the @aws-cdk/aws-stepfunctions-tasks label Nov 19, 2020

shivlaks self-requested a review November 19, 2020 00:08

pahud mentioned this pull request Nov 19, 2020

[stepfunctions-tasks] API Gateway support #11566

Closed

2 tasks

pahud reviewed Nov 19, 2020

View reviewed changes

ayush987goyal suggested changes Nov 19, 2020

View reviewed changes

shivlaks previously requested changes Nov 19, 2020

View reviewed changes

Use IRestApi & resolve other comments

fbb40d6

Sumeet Badyal added 5 commits November 19, 2020 12:18

update README.md

e14080a

Make comments for HttpMethod and HttpMethod.GET single line

32d0e26

linting errors

494334b

Fix waitForTaskToken unit test

ac29551

Add aws-apigateway to package.json

c47bb7e

ayush987goyal suggested changes Nov 20, 2020

View reviewed changes

shivlaks previously requested changes Nov 25, 2020

View reviewed changes

Remove apiEndpoint and other comments

9cd40c7

nija-at assigned nija-at and unassigned nija-at Dec 8, 2020

ayush987goyal mentioned this pull request Feb 14, 2021

feat(stepfunctions-tasks): Support calling ApiGateway REST and HTTP APIs #13033

Merged

shivlaks closed this Mar 3, 2021

dependabot bot mentioned this pull request Mar 12, 2021

build(deps): bump aws-cdk from 1.92.0 to 1.93.0 linz/geostore#393

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(stepfunctions-tasks): support for APIGW API: Invoke #11565

feat(stepfunctions-tasks): support for APIGW API: Invoke #11565

Sumeet-Badyal commented Nov 19, 2020 •

edited by shivlaks

Loading

gitpod-io bot commented Nov 19, 2020 •

edited

Loading

shivlaks commented Nov 19, 2020

ayush987goyal left a comment

shivlaks Nov 19, 2020

shivlaks Nov 19, 2020

ayush987goyal Nov 20, 2020

shivlaks left a comment

shivlaks Nov 23, 2020

shivlaks Nov 23, 2020

shivlaks Nov 23, 2020

eikeon Dec 7, 2020

shivlaks Dec 9, 2020

eikeon Dec 30, 2020

eikeon Dec 30, 2020

eikeon Dec 30, 2020

eikeon Dec 30, 2020

eikeon Dec 30, 2020

shivlaks Nov 23, 2020

shivlaks Nov 23, 2020

shivlaks Nov 25, 2020

shivlaks Nov 25, 2020

shivlaks Nov 25, 2020

shivlaks Nov 25, 2020

shivlaks Nov 25, 2020

Sumeet-Badyal Dec 3, 2020

aws-cdk-automation commented Dec 3, 2020

ghost commented Jan 17, 2021

shivlaks commented Mar 3, 2021

feat(stepfunctions-tasks): support for APIGW API: Invoke #11565

feat(stepfunctions-tasks): support for APIGW API: Invoke #11565

Conversation

Sumeet-Badyal commented Nov 19, 2020 • edited by shivlaks Loading

gitpod-io bot commented Nov 19, 2020 • edited Loading

shivlaks commented Nov 19, 2020

ayush987goyal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shivlaks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented Dec 3, 2020

AWS CodeBuild CI Report

ghost commented Jan 17, 2021

shivlaks commented Mar 3, 2021

Sumeet-Badyal commented Nov 19, 2020 •

edited by shivlaks

Loading

gitpod-io bot commented Nov 19, 2020 •

edited

Loading